feat: Added statusCodesToTreatAsSuccess (additive success set) to NewBatchWithFailedRequests and Preserving RequestId - #980
Conversation
|
@microsoft-github-policy-service agree |
|
Jeppe Mastrup Spanggaard (@jeppesc11) Can you provide reasons or issue reference for added context as to why we would want to maintain the same request id? |
|
Ramses Sanchez-Hernandez (@ramsessanchez) I can see that I should have explained why - apologies. The reason is that, for example, with “Get file content” you don’t get an ID back that you can use to correlate with your requests -only the batch request ID. So if some number of the 20 calls in the batch fail and I then retry, I no longer have a way to correlate request and response. I have previously written a longer blog post about this exact issue.
The reason for statusCodesToTreatAsSuccess is, again, that with “Get File Content” about 9 out of 10 times I get a response code indicating Redirect or Found. That means these will, by default, be retried - even though it’s a valid status code for this scenario. The same applies to 404. |
Jeppe Mastrup Spanggaard (jeppesc11)
left a comment
Jeppe Mastrup Spanggaard (jeppesc11)
left a comment
There was a problem hiding this comment.
Looks fine with the changes made.
Jeppe Mastrup Spanggaard (jeppesc11)
left a comment
Jeppe Mastrup Spanggaard (jeppesc11)
left a comment
There was a problem hiding this comment.
Looks fine
Address review feedback: - NewBatchWithFailedRequests now requires the caller to pass a HashSet<HttpStatusCode>, removing the IEnumerable-to-HashSet conversion inside the method. - Collapse the IsSuccess local function into an expression body. Parentheses around the null-coalescing operand are required: without them `a || set?.Contains(code) ?? false` parses as `(a || bool?) ?? false` and fails to compile with CS0019. - The single-argument overload now delegates to the two-argument one so the RequestId-preserving step construction lives in one place.
3 participants
Changes proposed in this pull request
NewBatchWithFailedRequestsNewBatchWithFailedRequestsMicrosoft Reviewers: Open in CodeFlow